Thanks for making this change! Looks good to me, but do we maybe want to change crl_secret to something like:

crl:
  secretName: string

Instead so that way we are leaving ourselves room for additional properties/fields should we add additional fields around the CRLs? I know Envoy exposes multiple options for handling them, so I could see us wanting to add more CRL fields to the CRDs in the future. @LanceEa Thoughts?

Good call @AliceProxy, I would agree that we should set ourselves up for future enhancements. In general, we need to start planning out our road to a proper CRD V3.

I would also say at a minimum we should be consistent with the casings of our fields. Based on our docs it should be camelCasing, correct?

I agree with you @AliceProxy and @LanceEa. tls.crl.secretName would be desirable both in terms of casing and consistency with how CRDs should be defined. We should aim for this in v3. However, I took a look at the other existing TLS fields:

tls:
  cert_chain_file:    # string
  private_key_file:   # string
  ca_secret:          # string
  cacert_chain_file:  # string
  alpn_protocols:     # string
  cert_required:      # bool
  min_tls_version:    # string
  max_tls_version:    # string
  cipher_suites:      # array of strings
  ecdh_curves:        # array of strings
  sni:                # string

So I went for consistency in the way current resources are defined... snake case and _secret suffix.
Does it make sense?

@alexgervais @LanceEa

I would also say at a minimum we should be consistent with the casings of our fields. Based on our docs it should be camelCasing, correct?

So one of our goals for the V3 CRDs is to move everything to camelCase since we currently have a mix of snake_case and camelCase all over the CRDs and camelCase is more "kubelike". Having it be snake_case for this PR not a big problem because with the V3 CRDs we can convert it to camelCase.

If we want to go with snake_case for this version, then I still think it would be beneficial to do something like:

crls:
   secret_name: string

because if we add new fields in the V3 CRDs, then we have to support those fields in the older CRD versions for round tripping conversion and that would make it simpler to add values into the object instead of having a V3 CRD that looks like:

  crls:
    secretName: string
    newField1: string
    newField2: string

and then have the v3alpha1/v2 CRDs look like this after conversion if we merge this as crl_secret: string:

  crl_secret: string
  new_field_1:  string
  new_field_2: string

Does that reasoning make sense?

As for the casing, we currently have some CRDs that use all snake_case, some that use all camelCase, and some that have a mix, so I'm not as picky about the casing for this PR since we will be standardizing the casing in the V3 CRDs.

Thanks for all your thoughts. I'm very much aligned with the CRD design for v3.
As for the current version, I still think we should keep the tls config block flat, as other keys also reference either a _secret or _file.
I am slightly afraid people would think of using the following example as an array of CRLs, even tho only 1 entry can be specified (per CA/Host/TLSContext).

crls:
   secret_name: string

Seeing how things are handled in the python code, I saw examples where we would take the CA chain from either a _secret or _file reference. If we need to extend the CRL to file, we could reuse that logic.

@AliceProxy - I'm ok with @alexgervais reasoning but I haven't felt all the pains the team might have in the past so If you feel strong about this then try to convince him otherwise 😄 . If not let's move forward and get this merged so we can focus on release activities.